feat(config): layered config tool filter — config_denied over user toggles#468
Open
nlaurance wants to merge 9 commits into
Open
feat(config): layered config tool filter — config_denied over user toggles#468nlaurance wants to merge 9 commits into
nlaurance wants to merge 9 commits into
Conversation
…enylist Adds two mutually exclusive fields to ServerConfig that let operators declare tool visibility statically in mcp_config.json rather than having to call the API or CLI after every fresh install. enabled_tools: ["list_issues", "get_issue"] // allowlist — only these visible disabled_tools: ["delete_repo", "force_push"] // denylist — hide these, allow rest Config validation rejects a server that has both fields set. On every applyDifferentialToolUpdate (server connect / tool refresh), applyConfigToolFilter walks the in-memory config, computes the desired enabled/disabled state for each discovered tool, and calls setToolEnabledNoEmit to persist it in BBolt. All existing enforcement paths (isToolCallable, retrieve_tools pre-ranking, call_tool_*) pick up the change automatically with no further modifications. Five unit tests cover: allowlist disables unlisted tools, allowlist re-enables a tool moved back into the list, denylist disables listed tools, no-op when neither field is set, and end-to-end integration through applyDifferentialToolUpdate. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Member
|
Thanks @nlaurance — clean implementation and genuinely thorough tests. I support the idea of config-based tool filtering — declaring tool visibility in My concern is purely the interaction model with the per-tool enable/disable that recently landed (#463 Web UI, #447, REST/CLI
Definition I'd like us to agree onFor me, config-based filtering should mean hard-off for unwanted tools, and for that to be solid it has to be transparent, predictable, and surfaced in the UI and CLI — never a silent toggle that snaps back. Precisely:
Spec use cases
Implementation-wise this means config should override the stored DB enable state at evaluation time rather than overwriting it: evaluate the config layer in Does this definition work for you? If we're aligned on it, I'm happy to pair on this PR and convert the current behavior to override the DB option after config read along these lines — the config fields + mutual-exclusion validation you've already written are solid and stay as-is. |
…lConfigDenied Replace the BBolt-writing applyConfigToolFilter function (which overwrote user preferences on every reconnect, emitted spurious audit events, and had no provenance) with an evaluation-time IsToolConfigDenied method that: - Reads config at call time, never writes to BBolt - Preserves user-set tool preferences and audit trail - Enables separation of concerns: config vs user intent Key changes: - Delete applyConfigToolFilter from tool_quarantine.go (63 lines removed) - Add IsToolConfigDenied(serverName, toolName string) bool to Runtime - Remove applyConfigToolFilter call from lifecycle.go - Rewrite tool_config_filter_test.go: 5 new tests for IsToolConfigDenied * AllowList: tools not listed are denied * DenyList: listed tools are denied * NoFilter: all tools allowed when config has no filter * UnknownServer: returns false for missing servers * UserDisabledPreserved: BBolt state is independent from config layer All 198 runtime tests pass. No behavior change to actual tool visibility— the config layer is now just evaluated at call time instead of persisted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add IsToolConfigDenied delegation on *Server and insert a config-layer check in isToolCallable so tools denied by enabled_tools/disabled_tools in the server config are hard-off at MCP call time, evaluated at runtime without touching BBolt storage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fig-denied tools - Add ConfigDenied bool field to contracts.Tool (json: config_denied,omitempty) - Enrich config_denied in handleGetServerTools via IsToolConfigDenied interface - Return HTTP 409 in handleSetToolEnabled when req.Enabled is true for a config-denied tool - Remove debug fmt.Printf lines from enrichment loop; use logger.Debug instead Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extended the Tool interface to include runtime fields that the backend sends and Vue components use: server_name, schema, usage, last_used, approval_status, disabled, and config_denied. This allows proper typing of these fields in the frontend instead of using unsafe casts. Simplified type assertions in isToolConfigDenied and isToolEnabled functions to use the properly-typed Tool interface directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
nlaurance
changed the title
Author
|
now rebased with changes suggested |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
applyConfigToolFilter(which overwrote user-ownedDisabledflags on every reconnect) with a pure evaluation-time config layerenabled_tools/disabled_toolsinmcp_config.jsonare now enforced without touching user preferences, emitting spurious audit events, or silently re-enabling tools the user manually disabledWhat changed
Backend
ServerConfig.IsToolAllowedByConfig(toolName)— pure config helperRuntime.IsToolConfigDenied(serverName, toolName)— evaluation-time check, no BBolt writesisToolCallable(MCP enforcement chokepoint) checks config layer before BBoltrecord.Disabled— config-denied tools are hard off across all paths (call_tool_*,retrieve_tools,upstream_serverstool counts)SetAllToolsEnabledskips config-denied tools so bulk Enable All never sets a misleadingDisabled=falseon a hard-off toolGET /api/v1/servers/{id}/toolsexposesconfig_denied: trueper toolPOST /api/v1/servers/{id}/tools/{tool}/enabledreturns HTTP 409 when trying to enable a config-denied toolUpstreamRecordnow persistsEnabledTools/DisabledToolsthrough BBolt round-trips (bug fix)Frontend
config_denied: trueshow alocked by configbadge and a read-only label instead of a toggleToolinterface inapi.tsnow properly typesdisabled,config_denied,approval_status, etc.Effective visibility model
Config can only restrict further — it can never force a tool back on against the user.
Test Plan
go test ./internal/config/... ./internal/runtime/... ./internal/server/... ./internal/httpapi/... ./internal/storage/...— all passcd frontend && npm run build— clean buildenabled_tools: ["list_issues"]→create_issueunreachable via MCP, shows locked in UIdisabled_tools: ["delete_repo"]→delete_repounreachable, locked in UIPOST /api/v1/servers/{id}/tools/delete_repo/enabledwith{"enabled": true}→ 409 Conflict🤖 Generated with Claude Code